Skip to content

[CI] Added Serial Execution Engine Test #3069

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AlexandreSinger
Copy link
Contributor

Since the CI always installs oneTBB and the execution engine is set to auto, I found that the CI always tested with the tbb execution engine. Some users may not have oneTBB installed for one reason or another and we need to ensure that VTR always builds.

Added a CI test which sets the parallel execution engine to serial for Tatum and VPR.

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code infra Project Infrastructure labels May 21, 2025
Copy link
Contributor

@AmirhosseinPoolad AmirhosseinPoolad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @AlexandreSinger. Agreed with having this test since we were basically leaving parts of the code base completely untested. I had one tiny nitpick about the name of the test, LGTM other than that.

Comment on lines 22 to 25

#ifdef VPR_USE_TBB
#include "stats.h"
#endif // VPR_USE_TBB
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a comment, just a question: Since get_num_bends_and_length was used even if VPR_USE_TBB was 0, does VPR just fail to compile without TBB right now or is it included in another header?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did. Thats what notified me that this was untested.

I actually hotfixed this issue in a separate PR: #3067

This PR still needs to be rebased.

Since the CI always installs oneTBB and the execution engine is set to
auto, I found that the CI always tested with the tbb execution engine.
Some users may not have oneTBB installed for one reason or another and
we need to ensure that VTR always builds.

Added a CI test which sets the parallel execution engine to serial for
Tatum and VPR.
@AlexandreSinger AlexandreSinger force-pushed the feature-ci-test-serial-engine branch from 9888bf9 to f2290ca Compare May 22, 2025 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra Project Infrastructure lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants